Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

task/WG-295-WG-179-WG-185: fix and improve projects with duplicate system path constraints #216

Conversation

nathanfranklin
Copy link
Collaborator

@nathanfranklin nathanfranklin commented Sep 6, 2024

Overview:

This PR does the following for users:

This PR also refactors the code by:

  • Dropping the use of ObservableProject and adding a watch_users and watch_content to Project WG-185
  • Improves nginx conf so we see the 409 error in local development.

Related to TACC-Cloud/hazmapper#255

Related Jira tickets:

Summary of Changes:

Testing Steps:

  1. Test with task/WG-295: improve error message when duplicate syncing project hazmapper#255
  2. Try creating multiple maps where Sync Folder is not checked. Having two maps in the same directory should now be allowed
  3. Try creating two maps for the same folder and have Sync Folder checked. The creation of the second map should fail.

Notes:

TODO

  • separate syncing users and syncing files
  • add script to confirm that migration works and everything looks right.
  • try locally with production and staging data to ensure migration is functioning.

Makefile Show resolved Hide resolved
geoapi/services/projects.py Outdated Show resolved Hide resolved
@taoteg
Copy link
Contributor

taoteg commented Sep 11, 2024

We should add logic to the backend routes that prevents a user from creating a new map in a project that is already syncing (even though we do not expose this on the client-side front-end, a user could still call the API and create a map there that violates our syncing constraints).

geoapi/tests/conftest.py Outdated Show resolved Hide resolved
geoapi/celery_app.py Outdated Show resolved Hide resolved
geoapi/services/projects.py Outdated Show resolved Hide resolved
Copy link
Contributor

@taoteg taoteg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic all LGTM. I left a slew of comments related to the naming-of-things. Do with them what you wish.

@nathanfranklin
Copy link
Collaborator Author

We should add logic to the backend routes that prevents a user from creating a new map in a project that is already syncing (even though we do not expose this on the client-side front-end, a user could still call the API and create a map there that violates our syncing constraints).

Addressed. Turns out we only allow name, description and public to be updated. I added a note to the docstr.

Copy link
Contributor

@sophia-massie sophia-massie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and working as expected. LGTM

@nathanfranklin nathanfranklin merged commit 7c10073 into main Sep 17, 2024
3 checks passed
@nathanfranklin nathanfranklin deleted the task/WG-295-WG-179-WG-185-fix-and-improve-projects-with-duplicate-system-path-constraints branch September 17, 2024 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants